feat(macos): experimental ProRes VideoToolbox encoder (opt-in, additive protocol)#5202
feat(macos): experimental ProRes VideoToolbox encoder (opt-in, additive protocol)#5202Nottlespike wants to merge 12 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds disabled-by-default experimental ProRes (VideoToolbox / macOS) support end-to-end, including server-side protocol advertisement, configuration/UI plumbing, and a new ScreenCaptureKit-based capture backend for modern macOS.
Changes:
- Introduces ProRes as a new video format with config gating (
prores_mode,prores_profile) and RTSP/NVHTTP advertisement. - Extends encoder probing/capabilities and pixel-format plumbing (NV24/P410) to support ProRes input requirements.
- Adds macOS ScreenCaptureKit capture backend and wires it into display selection for macOS 12.3+.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_video.cpp | Adds unit tests for ProRes config parsing and format gating helpers. |
| src_assets/common/assets/web/public/assets/locale/en.json | Adds UI strings for ProRes mode/profile settings. |
| src_assets/common/assets/web/configs/tabs/encoders/VideotoolboxEncoder.vue | Adds ProRes mode/profile controls to VideoToolbox encoder tab. |
| src_assets/common/assets/web/config.html | Adds default config values for prores_mode and prores_profile. |
| src/video.h | Defines format constants, adds ProRes codec slot, and helper predicates. |
| src/video.cpp | Implements ProRes profile mapping, probing, encoder selection constraints, and new pixel formats. |
| src/rtsp.cpp | Advertises ProRes support via SDP and validates/gates client videoFormat requests. |
| src/platform/windows/display_vram.cpp | Updates format comparisons to named constants. |
| src/platform/macos/sc_video.m | Adds ScreenCaptureKit-based capture implementation with HDR gating support. |
| src/platform/macos/sc_video.h | Declares SCVideo and its capture protocol conformance. |
| src/platform/macos/nv12_zero_device.cpp | Maps new NV24/P410 pixel formats to CVPixelBuffer types for macOS capture. |
| src/platform/macos/display.mm | Switches capture backend to SCK on macOS 12.3+ and generalizes capture pointer type. |
| src/platform/macos/av_video.h | Introduces SunshineVideoCapture protocol shared by AVVideo/SCVideo. |
| src/platform/common.h | Adds nv24/p410 to pix_fmt enumeration and string conversion. |
| src/nvhttp.cpp | Adds optional NVHTTP fields for experimental ProRes capability/profile. |
| src/nvenc/nvenc_base.cpp | Extends codec string logging to include ProRes. |
| src/config.h | Adds apply_config(...) declaration and new ProRes config fields. |
| src/config.cpp | Adds ProRes profile normalization and parses prores_mode/prores_profile. |
| docs/configuration.md | Documents prores_mode and prores_profile configuration options. |
| docs/changelog.md | Notes experimental macOS ProRes support addition in Unreleased section. |
| cmake/dependencies/macos.cmake | Requires ScreenCaptureKit framework at configure time. |
| cmake/compile_definitions/macos.cmake | Links ScreenCaptureKit and compiles sc_video.m with ARC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| active_prores_mode = config::video.prores_mode; | ||
| // Bind `require_prores` to the user-configured value, NOT to the | ||
| // mutable `active_prores_mode` global. `adjust_encoder_constraints` | ||
| // below may demote `active_prores_mode` to 0 when no encoder supports | ||
| // ProRes; reading from the immutable config source keeps the | ||
| // "user explicitly asked for forced ProRes" intent intact across that | ||
| // demotion, so we can fail loudly instead of silently picking a | ||
| // non-ProRes encoder. | ||
| const bool require_prores = config::video.prores_mode >= 2; |
| if (video::active_prores_mode > 0) { | ||
| ss << "a=x-sunshine-prores:1"sv << std::endl; | ||
| ss << "a=x-sunshine-prores-profile:"sv << config::video.prores_profile << std::endl; | ||
| ss << "a=rtpmap:99 prores/90000"sv << std::endl; | ||
| } |
| - (void)dealloc { | ||
| BOOL running; | ||
| SCStream *stream; | ||
| @synchronized(self) { | ||
| running = self.streamRunning; | ||
| stream = self.stream; | ||
| self.streamRunning = NO; | ||
| self.currentCallback = nil; | ||
| } | ||
| if (running && stream) { | ||
| // Best-effort synchronous stop with a bounded wait so a | ||
| // misbehaving SCK doesn't hang teardown. | ||
| dispatch_semaphore_t stopped = dispatch_semaphore_create(0); | ||
| [stream stopCaptureWithCompletionHandler:^(NSError *_Nullable error) { | ||
| (void) error; | ||
| dispatch_semaphore_signal(stopped); | ||
| }]; | ||
| dispatch_semaphore_wait(stopped, dispatch_time(DISPATCH_TIME_NOW, 2 * NSEC_PER_SEC)); | ||
| } | ||
| } |
| auto device = std::make_unique<nv12_zero_device>(); | ||
|
|
||
| device->init(static_cast<void *>(av_capture), pix_fmt, setResolution, setPixelFormat); | ||
| device->init((void *) av_capture, pix_fmt, setResolution, setPixelFormat); |
| static_assert(video::SUNSHINE_FORMAT_H264 == 0); | ||
| static_assert(video::SUNSHINE_FORMAT_HEVC == 1); | ||
| static_assert(video::SUNSHINE_FORMAT_AV1 == 2); | ||
| static_assert(video::SUNSHINE_FORMAT_PRORES == 3); | ||
| static_assert(video::SUNSHINE_FORMAT_COUNT == 4); | ||
|
|
208f61f to
f89e868
Compare
|
@andygrundman re: "how are you viewing this stream on the client side?" — testing with a custom Moonlight build that has the ProRes decode path wired (stock Moonlight has no ProRes decoder, which is why this PR is opt-in via `prores_mode` and the protocol negotiation is additive — a stock Moonlight client never sees ProRes signaled). The intent is to leave the door open for custom clients that want lossless-ish desktop streaming on LAN — capture workflows, A/B tests against HEVC, etc. — without affecting the default streaming path at all. |
f89e868 to
eedcf8f
Compare
|
Force-pushed ( Re Copilot's specific findings on this PR:
Let me know on the platform guard / SDP whitelist / test cleanup preferences; happy to do them in a follow-up commit on this PR. |
…12.3+
AVCaptureScreenInput was deprecated in macOS 13 (October 2022) and is
fundamentally limited to 8-bit BGRA, blocking any honest HDR or 10-bit
work on the macOS capture path. ScreenCaptureKit has been available
since macOS 12.3 (March 2022) and is the only forward path; this
commit lays the foundation by adding a drop-in SCK-based backend that
preserves behaviour exactly (same pixel format, frame rate, display
selection) so it can be reviewed independently of the HDR work that
builds on top.
Changes:
* Add SunshineVideoCapture protocol in av_video.h declaring the
capture-side surface both backends expose.
* Make AVVideo conform to the protocol (no behaviour change; pure
declaration).
* Add SCVideo (sc_video.h / sc_video.m) implementing the same
protocol against SCStream + SCContentFilter + SCStreamConfiguration.
Built with -fobjc-arc for SCK's block-heavy API surface; objects
cross the MRC boundary via the standard +1-retain alloc/init
convention so display.mm continues to work in MRC.
* Drop incomplete frames from SCK output by inspecting
SCStreamFrameInfoStatus on each sample-buffer attachment, matching
the reliability the legacy path got for free from AVCaptureSession.
* display.mm now holds an id<SunshineVideoCapture> and branches at
construction via @available(macOS 12.3, *): SCVideo on supported
systems, AVVideo as fallback for older macOS.
* Wire ScreenCaptureKit framework into cmake/dependencies/macos.cmake
and cmake/compile_definitions/macos.cmake; set ARC compile flag on
sc_video.m only.
Pixel format stays 32BGRA for this commit; 10-bit + EDR metadata
follow in a subsequent change.
…pixel formats
With AVCaptureScreenInput, asking the capture surface for a 10-bit
pixel format silently produced 8-bit BGRA — the OS-level lie that
made HEVC Main10 / AV1 Main10 / ProRes 10-bit profiles on macOS into
fake HDR (color-tagged 8-bit data). With ScreenCaptureKit landing in
the previous commit, 10-bit pixel formats are actually honoured, but
SCK needs an explicit signal to attach HDR metadata to those buffers
instead of treating them as 10-bit Rec.709.
This commit wires SCStreamConfiguration.captureDynamicRange:
* Add +pixelFormatIsHighBitDepth: classifier covering the YUV 4:2:0,
4:2:2 and 4:4:4 10-bit BiPlanar formats plus ARGB2101010 packed
and 64-bit RGBA formats.
* On the synchronous init path, set captureDynamicRange immediately
if the starting pixel format is high bit depth so the very first
sample buffer carries HDR metadata.
* On the setPixelFormat: path (called by nv12_zero_device when the
encoder selects p010), also update captureDynamicRange and push
the new config to a running stream via -updateConfiguration:.
* Use SCCaptureDynamicRangeHDRLocalDisplay rather than canonical
HDR: game streaming wants the host display's actual HDR
characteristics (peak luminance, primaries) so the receiver shows
what a local user would see, not Apple's idealised reference.
* Guard the whole block behind @available(macOS 14.0, *); on
12.3-13.x SCK still honours the 10-bit pixel format request but
doesn't auto-tag buffers, so Sunshine's existing colorspace logic
continues to drive the encoder's color fields.
Validated on M4 Max: Sunshine's encoder probe matrix now includes
successful 10-bit HEVC and 10-bit ProRes entries that previously
could not have validated because the capture surface couldn't
deliver matching pixel data. ProRes-specific VideoToolbox color tags
land in a separate follow-up commit.
…session HDR Two improvements to the macOS ScreenCaptureKit backend that landed in the upstream review cycle (LizardByte#5190) and never made it back onto our fork's dev: 1. **Lifecycle hardening**: - Register the SCStreamOutput exactly once in -init, not on every -capture: call. SCStream retains outputs across stop/start cycles, so re-registering would fail or silently duplicate delivery. - Bound all SCK completion-handler waits to 5s. SCK should always invoke them, but a misbehaving system service must not hang the whole startup path. - @synchronized(self) around all reads/writes of currentCallback / currentSignal / streamRunning. The sample-handler queue, the -capture: caller, and the SCStream delegate all touch these from different threads. - dispatch_queue_attr_make_with_qos_class's third argument is a RELATIVE priority (range -15..0), not one of the legacy DISPATCH_QUEUE_PRIORITY_* constants. Using 0 keeps the queue at its QoS class's nominal priority. - CGDisplayBounds fallback when CGDisplayCopyDisplayMode returns NULL (display reconfiguration races). 2. **EDR gating fix**: The previous EDR code flipped captureDynamicRange = HDRLocalDisplay whenever the chosen CVPixelBuffer format was 10-bit. That's necessary but not sufficient: a 10-bit format may be selected for codec reasons (e.g., a ProRes profile that requires 4:4:4 10-bit input) without the client ever requesting HDR ingest. Without gating, Sunshine would tell the client "HDR mode false" in the SDP while emitting BT.2020 PQ-tagged buffers — a silent control/data-plane mismatch. Now EDR requires BOTH 10-bit pixel format AND the negotiated session's enable_hdr (plumbed from launch_session_t via the existing config.dynamicRange field on video::config_t). Default is SDR; HDR is opt-in per session. New init signature: initWithDisplay:frameRate:hdrAllowed: The old initializer is preserved as a convenience that passes NO. New log line at session start makes the gating visible: "Using ScreenCaptureKit capture backend (HDR allowed|blocked)"
FIND_LIBRARY(SCREEN_CAPTURE_KIT_LIBRARY ScreenCaptureKit REQUIRED) so configure fails fast on environments without the SDK rather than later at header-lookup time. sc_video.m is compiled unconditionally; the REQUIRED keyword ensures the build prerequisites are surfaced clearly when the build host's Xcode/SDK is older than 13.3 / 12.3 (well past routine compatibility). Addresses Copilot inline feedback from the closed upstream PR cycle.
ScreenCaptureKit became available in macOS 12.3; Sunshine's deployment target (MACOSX_DEPLOYMENT_TARGET=14.2) is well above that. The @available(macOS 12.3, *) runtime branch in display.mm and the entire AVCaptureScreenInput-based AVVideo class were therefore dead code on every supported build. Changes: - Remove @available(macOS 12.3, *) check in display.mm; SCK is the only branch. - Replace `id<SunshineVideoCapture>` with `SCVideo *` directly — the protocol existed to abstract over both AVVideo and SCVideo, and is no longer needed with a single concrete capture class. - Move the small bits we still need (FrameCallbackBlock typedef, +displayNames / +getDisplayName: helpers) from av_video.{h,m} into sc_video.{h,m}. - Delete src/platform/macos/av_video.{h,m} (208 lines). - Drop both from PLATFORM_TARGET_FILES. Addresses andygrundman + ReenigneArcher review feedback on the original PR: "shouldn't keep workarounds for versions older than what we support."
Two Copilot findings on the SCK PR: 1. **dealloc signals pending semaphore.** -dealloc was clearing currentCallback but never signalling currentSignal. If the stream stopped without firing -stream:didStopWithError:, any caller still waiting on the semaphore returned by -capture: would stall forever. Snapshot the pending signal in the @synchronized block, then send it after clearing the callback so the waiter wakes up to observe their callback is nil and exits. 2. **Drop unused streamOutputAdded property.** Set in -init but never read. Removed both the @Property declaration and the assignment. The "register output exactly once at init" invariant is now structural (the call is in -init, not gated on a flag) and the dead state can't drift.
- Introduced `prores_mode` configuration option to enable custom clients to request ProRes video streams. - Added `prores_profile` configuration option to set the FFmpeg ProRes profile. - Updated changelog to reflect the addition of ProRes encoder plumbing. - Enhanced configuration documentation with details on ProRes options. - Implemented necessary changes in the codebase to support ProRes encoding, including updates to video format handling and encoder capabilities. - Added tests to validate ProRes configuration and protocol behavior.
…olbox tab The experimental ProRes mode selector landed in the generic Advanced tab while prores_profile (its inseparable companion — the mode toggle only makes sense if you also pick a profile) lived on the VideoToolbox Encoder tab. Splitting the two knobs across tabs hurts discoverability: users configuring VideoToolbox don't realise ProRes exists, and users on the Advanced tab can't see the profile that controls bit depth and chroma subsampling. Move prores_mode into VideoToolboxEncoder.vue directly above prores_profile so both controls sit together in their codec-relevant section. The macOS-only platform guard previously around prores_mode in Advanced.vue is implicit on the VideoToolbox tab (which is itself macOS-only), so it can be dropped. No backend or config change; pure UI relocation.
ProRes profiles are intrinsically 10-bit (proxy / lt / standard / hq)
or 12-bit (4444 / 4444 XQ); FFmpeg's prores_videotoolbox supported
input pix_fmt list does not include the 4:2:0 BiPlanar formats (NV12 /
P010) Sunshine has historically delivered for H.264 / HEVC / AV1. The
422 family wants 4:2:2 or higher chroma and the 4444 family wants
4:4:4 natively. Until now, the macOS capture path could only produce
4:2:0 buffers, so the ProRes encoder probe failed at avcodec_open2
with "Couldn't open" regardless of profile selection or colorspace
configuration.
Wire the 4:4:4 BiPlanar capture path end-to-end:
* Add nv24 / p410 to platf::pix_fmt_e and the from_pix_fmt switch.
* Map AV_PIX_FMT_NV24 / AV_PIX_FMT_P410 in video::map_pix_fmt.
* Populate the previously-NONE 4:4:4 pix_fmt slots on the
videotoolbox encoder declaration and add YUV444_SUPPORT to its
flags. H.264 and HEVC VideoToolbox don't gain anything new
functionally — they remain 4:2:0 only on Apple Silicon hardware —
but the 4:4:4 probe runs against them harmlessly and falls
through with their YUV444 capability bit set false, which is
correct.
* Route nv24 / p410 through nv12_zero_device in
display.mm::make_avcodec_encode_device, alongside the existing
nv12 / p010 paths.
* Set the matching CVPixelBufferType
(kCVPixelFormatType_444YpCbCr*BiPlanarVideoRange) per pix_fmt in
nv12_zero_device::init.
Update the ProRes encoder probe in test_prores to use a ProRes-shaped
config:
* dynamicRange = 1 (10-bit pix_fmt slot instead of 8-bit).
* encoderCscMode = 3 (full range, BT.709 instead of BT.601).
* chromaSamplingType = 1 (4:4:4 P410 instead of 4:2:0 P010).
Any ProRes probe that succeeds inherently uses 10-bit input, so
DYNAMIC_RANGE is promoted eagerly here rather than relying on
test_hdr_and_yuv444 below (which gates on PASSED and only sets
DYNAMIC_RANGE itself).
Verified end-to-end on M4 Max: the startup probe now produces
"Found ProRes encoder: prores_videotoolbox [videotoolbox]", where
previously the encoder failed to open at every config permutation
attempted.
This is the runtime-side companion to the build-deps change that
adds prores_videotoolbox to the FFmpeg configure's --enable-encoder
flag (LizardByte/build-deps#693). Without that, the encoder symbol
is missing from libavcodec.a's static codec list and
avcodec_find_encoder_by_name returns null regardless of the wiring
in this commit.
video.h: is_known_video_format() now uses `< SUNSHINE_FORMAT_COUNT` instead of the hardcoded `<= SUNSHINE_FORMAT_PRORES` upper bound. Future codecs are now a one-enum-bump change. video.cpp: require_prores binds to the immutable config::video.prores_mode rather than the mutable active_prores_mode. adjust_encoder_constraints() may demote active_prores_mode to 0; reading the immutable source keeps the "user explicitly asked for forced ProRes" intent intact so we fail loudly instead of silently picking a non-ProRes encoder. cmake/dependencies/macos.cmake: Pull in the SCREEN_CAPTURE_KIT_LIBRARY REQUIRED change too — this PR stacks on the SCK backend PR and needs the same SDK enforcement. Addresses Copilot inline feedback from the closed upstream PR cycle.
Per Copilot review: tests/unit/test_video.cpp:10-13 duplicated the exact static_asserts that already live next to the enumerator declarations in src/video.h:28-31. Renumbering a format would require keeping the two lists in sync. Replace with a single test-relevant invariant — SUNSHINE_FORMAT_COUNT must equal PRORES + 1 — and link the comment back to where the authoritative assertions live. The H264/HEVC/AV1/PRORES==N value constraints stay in src/video.h where they belong (next to the constants themselves).
3e3cf99 to
208d956
Compare
…der enablement Carries the FFmpeg configure flag change (LizardByte/build-deps#693, commit c40fba6 "build: enable ProRes VideoToolbox encoder") that the ProRes runtime wiring in this PR depends on. The SHA a51ecf15 lives on the RESMP-DEV/build-deps fork branch feat/ffmpeg/enable-prores-videotoolbox-encoder — it is not yet on LizardByte/build-deps master because build-deps#693 is still open. Normally this would ship as a standalone `build(deps): bump third-party/build-deps` PR per the Sunshine convention (cf. LizardByte#5296, LizardByte#5280, LizardByte#5271), but those PRs always point at canonical master SHAs. Until build-deps#693 lands, this fork-scoped pointer is folded into the feature PR so reviewers can actually build and test the ProRes path. Once LizardByte#693 merges, replace a51ecf15 with the canonical master SHA via the standard dep-bump flow.
208d956 to
1ea1ac0
Compare
|




Reopens #5192 (closed prematurely; GitHub blocked direct reopen due to regenerated branch SHAs). All review feedback from the original cycle is addressed in additional commits. See the original PR for full discussion; rebased on current master.